Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(embedded): provides filter bar visibility setting on embedded dashboard (#21069) #21070

Conversation

jileeon
Copy link
Contributor

@jileeon jileeon commented Aug 12, 2022

SUMMARY

Provides filter bar visibility setting on embedded dashboards

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

@michael-s-molina
Copy link
Member

Thanks for the contribution @jileeon.

@lilykuang Do you think it makes sense to add these configurations under dashboardUiConfig? Something like:

dashboardUiConfig: {
   hideTitle: true,
   filters: {
      visible: true,
      expanded: true,
   }
}

Copy link
Member

@lilykuang lilykuang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michael-s-molina you suggestion will work better.

@jileeon jileeon force-pushed the feat-embebbed-sdk-add-dashboard-filter-config branch from 99c6063 to fb26de4 Compare August 18, 2022 02:46
@pull-request-size pull-request-size bot added size/M and removed size/S labels Aug 18, 2022
@jileeon
Copy link
Contributor Author

jileeon commented Aug 18, 2022

Hi @michael-s-molina,

I've changed the commit for your suggestion.

Thanks

@jileeon jileeon force-pushed the feat-embebbed-sdk-add-dashboard-filter-config branch from fb26de4 to f919201 Compare September 1, 2022 08:02
@jileeon
Copy link
Contributor Author

jileeon commented Sep 1, 2022

Hi, Sorry for the wrong constant values. I've corrected it now.
Please check the source code changes.

Thanks.

@michael-s-molina michael-s-molina merged commit eb80568 into apache:master Sep 1, 2022
@eschutho eschutho added the v2.0.1 label Sep 2, 2022
eschutho pushed a commit that referenced this pull request Sep 20, 2022
…shboard (#21069) (#21070)

Co-authored-by: 이재익 [jileeon] <jileeon@nexon.co.kr>
(cherry picked from commit eb80568)
filters?: {
[key: string]: boolean | undefined
visible?: boolean
expanded?: boolean
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jileeon - considering DASHBOARD_UI_FILTER_CONFIG_URL_PARAM_KEY only has 2 keys namely visible and expanded and line number 115 does DASHBOARD_UI_FILTER_CONFIG_URL_PARAM_KEY[key]=filterConfig[key], i don't understand why there is a need for line number 41 ([key: string]: boolean | undefined) in the code.

I found this code while I was trying to add some additional capability add some extra url parameters to the embedded superset. But I don't want to add that code if the functionality already exist.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @jayakrishnankk, Sorry for the late reply.

these code you wrote are just for complying with Typescript lint.
I think there are no need to keep them If they should be changed.

Thanks.

AAfghahi pushed a commit that referenced this pull request Oct 6, 2022
…shboard (#21069) (#21070)

Co-authored-by: 이재익 [jileeon] <jileeon@nexon.co.kr>
(cherry picked from commit eb80568)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants